Skip to content

Conversation

@ImeevMA
Copy link
Contributor

@ImeevMA ImeevMA commented Dec 5, 2025

This patch adds the push field to ExecuteRequest and introduces aeon_queue.proto module.

Needed for tarantool/aeon#488

@ImeevMA ImeevMA force-pushed the imeevma/gh-488-execute-push-field branch 2 times, most recently from e61b13b to b4137d8 Compare December 5, 2025 11:26
@ImeevMA ImeevMA requested review from locker and unera December 5, 2025 11:33
aeon_queue.proto Outdated
string consumer = 2;
// Max number of returned messages.
uint64 limit = 3;
// Time to live of the request.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is TTL of the consumer, not of the request.

Actually, all the comments are quite confusing. Please revise - write a few words so that it's clear what each is one is used for and what it should be set to.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. I've rewritten most of the comments and deleted the unnecessary ones.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still not good enough. We will ship this proto file to end users and it'd be nice if they could figure out how to use the API by just reading the comments. Try to pretend that you are a user unfamiliar with the implementation and write comments so that you can understand the purpose of each API function by just looking at them.

@locker locker assigned ImeevMA and unassigned unera and locker Dec 5, 2025
This patch adds the `push` field to `ExecuteRequest` and introduces
`aeon_queue.proto` module.

Needed for tarantool/aeon#488
@ImeevMA ImeevMA force-pushed the imeevma/gh-488-execute-push-field branch from b4137d8 to 21875fc Compare December 5, 2025 14:50
@ImeevMA ImeevMA requested a review from locker December 5, 2025 14:51
@ImeevMA ImeevMA assigned locker and unassigned ImeevMA Dec 5, 2025
// Time to live of the message.
double ttl = 3;
}
// Push function.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What arguments does this function take? In what language is it written? What is it supposed to return? Please write comments so that a user who isn't familiar with the implementation can figure out how to use this API.

message TakeMessagesRequest {
// Topic name.
string topic = 1;
// Unique consumer name.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's it used for?

// Takes messages from a shard-local queue.
rpc TakeMessages(TakeMessagesRequest) returns (TakeMessagesResponse) {}

// Releases messages.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens if they aren't released?

@locker locker assigned ImeevMA and unassigned locker Dec 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants